Refactor stream_readers and stream_writers into smaller files#816
Refactor stream_readers and stream_writers into smaller files#816mikeprosserni merged 6 commits intomasterfrom
Conversation
|
Just snooping, but... love this. Thank you! |
mikeprosserni
changed the title
mikeprosserni
requested review from
bkeryan,
maxxboehme and
zhindes
as code owners
There was a problem hiding this comment.
<nidaqmx.stream_writers.UnsetAutoStartSentinel object> changed to <nidaqmx.stream_writers._channel_writer_base.UnsetAutoStartSentinel object>
This sounds like compatibility breakage. Previously, clients could import UnsetAutoStartSentinel and AUTO_START_UNSET, and now they can't.
There was a problem hiding this comment.
Note that UnsetAutoStartSentinel is deleted right after being declared, so users could never import it, they could only import AUTO_START_UNSET itself.
class UnsetAutoStartSentinel:
pass
AUTO_START_UNSET = UnsetAutoStartSentinel()
del UnsetAutoStartSentinel
There was a problem hiding this comment.
This PR introduces public submodules, which introduces another way for clients to import these classes. Shouldn't the new submodules be private?
Most of the pain this causes is related to documentation, but it sounds like autodoc is handling this better than autoapi would.
There was a problem hiding this comment.
We used private submodules for task, etc.
3 participants
I've updated CHANGELOG.md if applicable.I've added tests applicable for this pull requestWhat does this Pull Request accomplish?
stream_readers.pyandstream_writers.pywere very large files, which made development and review more difficult. This PR splits the classes in them out into individual files to make things more manageable.I was careful to keep each class unchanged through this refactor. I copy-pasted each entire class and made no edits within the classes. The only changes would be in the imports sections.
Why should this Pull Request be merged?
This should make reviewing future changes easier.
What testing has been done?
stream_writers.htmltostream_writers/analog_multi_channel_writer.html<nidaqmx.stream_writers.UnsetAutoStartSentinel object>changed to<nidaqmx.stream_writers._channel_writer_base.UnsetAutoStartSentinel object>